Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Travis: Fix requirements install #5061

Merged
merged 1 commit into from
Sep 20, 2017
Merged

Travis: Fix requirements install #5061

merged 1 commit into from
Sep 20, 2017

Conversation

geky
Copy link
Contributor

@geky geky commented Sep 8, 2017

pip requires --user flag when installing executables outside of sudo context

see #5055
cc @0xc0170, @theotherjimmy

@geky geky changed the title Fix travis requirements install Travis: Fix requirements install Sep 8, 2017
@0xc0170
Copy link
Contributor

0xc0170 commented Sep 9, 2017

Is there any doc reference for this? Is also the fix referenced above needed or this supersedes it?

@geky
Copy link
Contributor Author

geky commented Sep 11, 2017 •
edited
Loading

So what I can find on the python side: https://www.python.org/dev/peps/pep-0370/

Basically, pip install tries to install in the python library path. This is usually in an admin-only location that requires sudo (default on ubuntu), but could be somewhere that every user has write access to if you like living life on the edge.

They added pip install --user for user-local installs which should never require sudo, but they couldn't change the default behavior for backwards compatibility.

Options are use sudo pip install or pip install --user. Unfortunately virtualenvs cause problems for sudo pip install, though I'm not aware of all the details. I ran into an issue earlier that travis wasn't putting sudo pip install in the python path, it's fixed now, but pip install --user seems more reliable.

Is also the fix referenced above needed or this supersedes it?

Good point, will remove

Nodraak
Nodraak reviewed Sep 12, 2017
View reviewed changes
.travis.yml
- pip install --user hypothesis
- pip install --user mock
- pip install --user coverage
- pip install --user coveralls
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not we have a requirements-travis.txt or requirements-test.txt file for theses? If not, why don't we install all packages in one pip install command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this list is big enough to need another level of indirection IMO.

I can run these all in one command though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, for some reason combining all of the requirements into one pip install command caused travis to fail. I couldn't figure out why.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am right, the error is from six import wraps - ImportError: cannot import name wraps.
Here I found that Mock requires six version 1.7 or newer.
I think you should add pip install --upgrade six to upgrade the already installed six to the newest version.

Anyway, I'm being picky, if it's good enough for mbed's maintainers, let's merge as it is :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, then why would pip work in this configuration?

Copy link
Contributor

@Nodraak Nodraak Sep 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum yeah, you are right ... no idea ¯(°_o)/¯

@geky geky force-pushed the geky/fix-travis branch 2 times, most recently from de8c730 to c10adfa Compare September 14, 2017 16:26
@geky
Fix travis requirements install
94c972e 
pip requires --user flag when installing executables outside of sudo context
@geky
Copy link
Contributor Author

geky commented Sep 18, 2017

@0xc0170, this is probably ready, won't need morph test either

@theotherjimmy
Copy link
Contributor

2X travis builds. This must be extra safe!

@theotherjimmy theotherjimmy merged commit b494d33 into master Sep 20, 2017
@geky geky deleted the geky/fix-travis branch September 26, 2017 18:11
geky added a commit to PelionIoT/sd-driver that referenced this pull request Sep 27, 2017
@geky
Fix .travis.yml requirements install
f32cad9 
Needs --user flag for reasons, see ARMmbed/mbed-os#5061

superseeds #60
cc @deepikabhavnani
@geky geky mentioned this pull request Sep 27, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nodraak Nodraak Nodraak left review comments

@0xc0170 0xc0170 0xc0170 approved these changes

Assignees
No one assigned
Labels
release-version: 5.6.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@geky @0xc0170 @theotherjimmy @Nodraak